feat(codegen): turn the segment-view lowering on by default - #9912
feat(codegen): turn the segment-view lowering on by default#9912proggeramlug wants to merge 1 commit into
Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jdalton
left a comment
There was a problem hiding this comment.
Review of 9b9aedda258ea678019667c4257f9642ffe1e8ce (2026-09-07).
Please add a compiled cache A/B regression for unset → PERRY_SEGVIEW=0 → unset, checking emitted calls/output rather than just the environment predicate. The new eligibility exclusion only covers the build-cache layer, so the object-cache fingerprint also needs to distinguish the setting; the comment still says the feature is default OFF and should be corrected for this PR. For the new cursor clear, add abrupt-control-flow cases (break and a labelled outer continue, plus a caught throw). A statement after the loop handles ordinary exit, but not every path that can keep the enclosing function/frame alive. No demonstrated semantic miscompile from the inspected diff.
Validation scope: source/diff inspection; I have not run this PR's build or test suite locally.
The tier shipped default OFF because the runtime's view entry points did not exist. PerryTS#9870 landed them on main, and on claude-code the tier with PerryTS#9893's levers is -14 % CPU and -30…-42 MB peak RSS across 5/5 paired runs with identical output. `PERRY_SEGVIEW=0` remains as the opt-out. A switch that changes emitted code needs an off position that does not require rebuilding the compiler, and a program whose loops the tier declines pays nothing either way. The test asserts the default through the same environment read the compiler uses. Without it, a predicate that was accidentally always-false would leave every other segview test passing — they call the rewrite directly — while the tier silently never fired in a real compile.
9b9aedd to
e495a1e
Compare
DRAFT. Gated on two things, neither of which is done:
PERRY_SEGVIEWinto the build-cacheidentity. Flipping the default while the switch is absent from that identity
would bake the hole into every build rather than every A/B — a cached
binary could be served for a source compiled under the other setting. fix(compile): put PERRY_SEGVIEW in the cache identity, and clear the segment-view cursor at loop exit #9910
is also the parent of this branch, so this diff is the one-line flip on top
of it.
set
PERRY_SEGVIEW=1explicitly. Default-on is a different configuration —it is what every compile in the tree does, including ones nobody chose it
for — and it has not been rotated. Until that row exists this is unmeasured
in the form it would ship.
The change
One line, plus the doc comment it invalidates:
PERRY_SEGVIEW=0becomes the opt-out. It stays because a switch that changesemitted code needs an off position that does not require rebuilding the
compiler; a program whose loops the tier declines pays nothing either way.
Why the default should move, once gated
On claude-code, with #9893's levers, paired against main on a quiet box:
−14 % CPU MIN (5/5 faster), −30…−42 MB peak RSS (5/5), settled RSS flat,
output identical; 4.8–5.3× node from main's 5.6–6.3×. On the probe's region A
— cc's per-
.segment()-call shape — 2,262 → 1,554 ns/grapheme against node's1,323, i.e. 1.17× node. Rows and their provenance are in #9859.
Without #9893's levers the same tier costs +15 % CPU, which is why the
gating on it is not a formality.
The test
Asserts the default through the same environment read the compiler uses: unset
⇒ on,
0⇒ off,1⇒ on. Every other segview test calls the rewrite directly,so a predicate that was accidentally always-false would leave all of them
passing while the tier silently never fired in a real compile — the #9824
shape, and the reason this assertion is worth its three lines.
18 segview tests pass on this branch.
Default-on gate on main
504e180d0(perrymaster, cc 3300-char reply, 5-round interleaved rotation; paired deltas)Identity from the artefacts: with
PERRY_SEGVIEWunset and no diag, the branch rewrites the same nine sites as the opt-in build (view call sites next/open 20/10 vs 0/0 on main), so the flip does exactly what the env switch did.400-char turn: default-on alone +0.09…+0.20 s (3/3 slower), with the levers −0.05…−0.11 s (3/3 faster); peak −9…−35 MB either way.
Consequence: this flip must land together with or after #9893. On its own it fails the campaign's CPU rule (+3…+5 % at 3300, +7…+14 % at 400) while buying peak RSS; with the levers it is the I7-view result on the current base. Absolute numbers on this host currently carry a ~35 % shift from a foreign service; the pairs above are interleaved and hold. Raw:
combDO.jsonl,idleDO.jsonl.Rebased onto main (2026-09-07)
The base PRs this branch was stacked on landed on main via merge train #9922, so the branch was rebased onto current main: head
9b9aedda2→e495a1e24, replay patch-identical (only this branch's own commits remain above main). The measurements above were taken on the pre-rebase head with the same code; CI on this head is the remaining gate.